Skip to content

fix: Add identity placeholders for existing principalId and clientId (#3972) - #3974

Open
Sebastian Claesson (SebastianClaesson) wants to merge 1 commit into
Azure:mainfrom
SebastianClaesson:fix/3972-unique-string-existing-principal
Open

Sebastian Claesson (SebastianClaesson) wants to merge 1 commit into
Azure:mainfrom
SebastianClaesson:fix/3972-unique-string-existing-principal

Conversation

@SebastianClaesson

Copy link
Copy Markdown

PR Summary

Fixes #3972.

Expanding a template fails when a nested module's name or a role assignment's name hashes the principalId (or clientId) of an existing user-assigned identity, for example uniqueString(grant.name, grant.principalId). The error is: "The argument 'additional parameters as needed' for 'UniqueString' is not a valid string", and the whole file is skipped.

Root cause: reference() on an existing identity that isn't part of the deployment returns an unknown mock for principalId/clientId. Once that mock is placed inside an object or array literal, such as a module parameter, it expands to null, so the nested uniqueString()/guid() gets a non-string.

Fix: add principalId and clientId of Microsoft.ManagedIdentity/userAssignedIdentities to the typed resource placeholder table introduced in #3908, so they resolve to a string. The placeholder is ffffffff-ffff-ffff-ffff-ffffffffffff, the value already used as PLACEHOLDER_GUID for identities deployed in the template, so existing and deployed identities now expand the same way. The expression functions stay strict, as in #3908.

Scope and trade-offs:

  • Every existing identity gets the same placeholder, as deployed identities already do. Two of them hashed against the same scope produce the same name, and PSRule merges the duplicate resource IDs rather than failing.
  • tenantId isn't added: its right value is the configured tenant, which the static table can't reach.
  • A system-assigned resource.identity.principalId goes through a different path and isn't covered here.

Tests:

  • New Tests.Bicep.46 fixture: an existing cross-resource-group identity, a module named ra-${uniqueString(grant.name, grant.principalId)}, and a role assignment named with guid(..., principalId, ...). Compiled with Bicep 0.48.1.
  • New TemplateVisitorTests.ProcessTemplate_WhenExistingResourcePropertyPassedToModuleAndHashed_ShouldExpand and FunctionTests.ReferenceWithIdentityPlaceholders. Both fail without the fix and pass with it.
  • BicepSymbolicNameTests: one pinned role assignment name changes because guid(principalId) now hashes the placeholder instead of an empty string.
  • Full .NET test suite: 410 passed locally.

PR Checklist

  • PR has a meaningful title
  • Summarized changes
  • Change is not breaking
  • This PR is ready to merge and is not Work in Progress
  • Other code changes
    • Unit tests created/ updated
    • Link to a filed issue
    • Change log has been updated with change under unreleased section

…zure#3972)

When a template references an `existing` user-assigned identity that is
not part of the deployment, `reference()` returns an unknown mock for its
`principalId` and `clientId`. Placed inside an object or array literal,
such as a module parameter, the unknown mock expands to `null`, so a
nested `uniqueString()` or `guid()` over the value failed with "The
argument 'additional parameters as needed' for 'UniqueString' is not a
valid string", and the whole file was skipped.

Add `principalId` and `clientId` of
`Microsoft.ManagedIdentity/userAssignedIdentities` to the typed resource
placeholder table added in Azure#3908, so they resolve to a string. The
placeholder is the same `ffffffff-ffff-ffff-ffff-ffffffffffff` GUID that
is already used for identities deployed in the template.

The pinned role assignment name in the symbolic name test changes because
`guid(principalId)` now hashes the placeholder instead of an empty string.

Fixes Azure#3972

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant